Skip to content

[#2441] Fixed Acquia DB download to skip auth header on S3 redirect.#2465

Merged
AlexSkrypnyk merged 1 commit intomainfrom
feature/2441-acquia-db-download
Apr 20, 2026
Merged

[#2441] Fixed Acquia DB download to skip auth header on S3 redirect.#2465
AlexSkrypnyk merged 1 commit intomainfrom
feature/2441-acquia-db-download

Conversation

@AlexSkrypnyk
Copy link
Copy Markdown
Member

@AlexSkrypnyk AlexSkrypnyk commented Apr 20, 2026

Closes #2441

Summary

Fixed two bugs in scripts/vortex/download-db-acquia.sh that caused Acquia DB downloads to fail.

The Acquia API actions/download endpoint returns a 302 redirect to a presigned S3 URL. The previous implementation followed the redirect with -L, which could send the Authorization: Bearer header to S3 - S3 presigned URLs reject requests that carry extra auth headers, causing the download to return an error body instead of the archive. The fix captures only the redirect URL (without following it) and then makes a separate, auth-free curl call to S3.

Additionally, error branches that deleted partially-downloaded or corrupt files with rm -f were changed to leave those files in place so they can be inspected when something goes wrong.

Changes

scripts/vortex/download-db-acquia.sh

  • Auth header leak fix: replaced curl --progress-bar -L ... | extract_json_value "url" with curl -s -o /dev/null -w "%{redirect_url}" ... to capture the S3 redirect URL without following it and without forwarding the Authorization header. The JSON-parsing branch was removed because the API only ever responds with a redirect, not a JSON body.
  • Preserve files on failure: removed three rm -f calls from error branches (empty/missing download, invalid gzip, decompression failure) so the corrupt file is kept for inspection.
  • Move cleanup to success path: moved rm "${file_name_compressed}" to after the decompression validity check so the compressed file is only deleted on a fully successful run.

.vortex/tests/bats/unit/download-db-acquia.bats

  • Updated three test-case curl mocks for the "Discovering backup URL" step to match the new curl -s -o /dev/null -w %{redirect_url} signature and to return a plain URL string instead of a JSON body.

Before / After

OLD flow

Acquia API call
  curl --progress-bar -L
       -H "Authorization: Bearer <token>"
       https://…/actions/download
    │
    │  302 redirect (followed by -L)
    ▼
  S3 presigned URL  ← Authorization header forwarded here
    │
    │  S3 rejects extra auth → returns error XML body
    ▼
  Corrupt/empty file saved locally
    │
    │  gzip check or size check fails
    ▼
  rm -f  ← evidence deleted, nothing to inspect

NEW flow

Acquia API call (capture redirect only)
  curl -s -o /dev/null -w "%{redirect_url}"
       -H "Authorization: Bearer <token>"
       https://…/actions/download
    │
    │  302 redirect URL extracted, connection closed
    ▼
  backup_url = "https://s3.amazonaws.com/…?X-Amz-Signature=…"

S3 download (no auth header)
  curl --progress-bar -L "${backup_url}"
    │
    │  S3 accepts presigned URL, streams archive
    ▼
  Valid .sql.gz file saved locally
    │
    ├─ success → gunzip → rm compressed file
    │
    └─ failure → files left in place for inspection

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced error diagnostics for Acquia database backup operations by preserving intermediate files for inspection when download or decompression fails, enabling improved troubleshooting
    • Optimized backup URL discovery mechanism
  • Tests

    • Updated test cases for Acquia backup URL discovery functionality

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 19a45ba7-ac87-4347-8bd8-1eb3ec82bb78

📥 Commits

Reviewing files that changed from the base of the PR and between c98ee30 and 292273e.

📒 Files selected for processing (2)
  • .vortex/tests/bats/unit/download-db-acquia.bats
  • scripts/vortex/download-db-acquia.sh

Walkthrough

The Acquia backup URL discovery logic is refactored to capture HTTP redirect targets directly via curl's -w %{redirect_url} flag instead of parsing JSON responses. Error handling is improved by preserving intermediate files (.gz) on failure for inspection, and only removing them explicitly after successful decompression. Corresponding test mocks are updated to reflect the new curl behavior.

Changes

Cohort / File(s) Summary
Test Mock Updates
.vortex/tests/bats/unit/download-db-acquia.bats
Updated mocked curl command expectations and responses in the "Discovering backup URL" test stage to match the new redirect URL extraction approach instead of JSON parsing.
URL Discovery and Error Handling
scripts/vortex/download-db-acquia.sh
Replaced JSON-based URL extraction with curl -w %{redirect_url} redirect capture. Removed file deletion logic on gzip validation and decompression errors to preserve artifacts for inspection; explicit cleanup now occurs only after successful decompression.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #1877: Directly overlaps with unit test updates and modifications to the Acquia download script.
  • PR #2120: Both PRs modify download-db-acquia.sh and its BATS tests to enhance Acquia backup discovery and error handling.
  • PR #2445: Both touch the Acquia DB download flow with curl usage and accompanying BATS test modifications.

Poem

🐰 A redirect captured with quiet finesse,
No JSON to parse, just the URL's address,
Failed files preserved for the detective to see,
Trust the artifacts—let them roam free! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main fix: preventing auth header forwarding to S3 redirect, which is the primary bug addressed in the PR.
Linked Issues check ✅ Passed The PR successfully addresses both requirements from issue #2441: capturing the redirect URL without forwarding auth headers, and leaving corrupt files for inspection instead of deleting them.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the two objectives in #2441: the shell script fix for auth header handling and the removal file deletion logic, plus corresponding test updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/2441-acquia-db-download

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

Code coverage (threshold: 90%)

  Classes: 100.00% (1/1)
  Methods: 100.00% (2/2)
  Lines:   98.53% (201/204)
Per-class coverage
Drupal\ys_demo\Plugin\Block\CounterBlock
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% ( 10/ 10)

@AlexSkrypnyk

This comment has been minimized.

2 similar comments
@AlexSkrypnyk

This comment has been minimized.

@AlexSkrypnyk
Copy link
Copy Markdown
Member Author

Code coverage (threshold: 90%)

  Classes: 100.00% (1/1)
  Methods: 100.00% (2/2)
  Lines:   98.53% (201/204)
Per-class coverage
Drupal\ys_demo\Plugin\Block\CounterBlock
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% ( 10/ 10)

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.45%. Comparing base (c98ee30) to head (292273e).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2465      +/-   ##
==========================================
- Coverage   79.88%   79.45%   -0.44%     
==========================================
  Files         128      121       -7     
  Lines        6861     6697     -164     
  Branches       47        3      -44     
==========================================
- Hits         5481     5321     -160     
+ Misses       1380     1376       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AlexSkrypnyk AlexSkrypnyk merged commit 7a3d7d3 into main Apr 20, 2026
30 checks passed
@github-project-automation github-project-automation bot moved this from BACKLOG to Release queue in Vortex Apr 20, 2026
@AlexSkrypnyk AlexSkrypnyk deleted the feature/2441-acquia-db-download branch April 20, 2026 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Release queue

Development

Successfully merging this pull request may close these issues.

Fix DB download from Acquia

1 participant